Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

go/badger: bump badger version #3142

Merged
merged 1 commit into from
Aug 3, 2020
Merged

go/badger: bump badger version #3142

merged 1 commit into from
Aug 3, 2020

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Jul 29, 2020 •
edited
Loading

TODO:

  • run the longtests and comapre cpu/memory/iops usage
    • looks much better during compaction & also fixes cpu spikes at node restarts (there was a bug with badger vlog replay that replayed all vlogs)
  • wait for the 72hours test to finish

@ptrus ptrus force-pushed the ptrus/feature/bump-badger branch 2 times, most recently from 87f9b07 to b32c6ff Compare July 29, 2020 14:32
@kostko
Copy link
Member

kostko commented Jul 29, 2020 •
edited
Loading

Looking at the failure, this may require disabling conflict detection by setting the WithDetectConflicts(false) (new option, previously this was implied by managed mode, but now it is not).

@ptrus ptrus force-pushed the ptrus/feature/bump-badger branch 2 times, most recently from 769e0d6 to 322aaf6 Compare July 29, 2020 15:22
@ptrus
Copy link
Member Author

ptrus commented Jul 29, 2020

Looking at the failure, this may require disabling conflict detection by setting the WithDetectConflicts(false) (new option, previously this was implied by managed mode, but now it is not).

That fixed the failure, thanks!

@codecov
Copy link

codecov bot commented Jul 29, 2020 •
edited
Loading

Codecov Report

Merging #3142 into master will increase coverage by 0.15%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3142      +/-   ##
==========================================
+ Coverage   68.72%   68.87%   +0.15%     
==========================================
  Files         383      383              
  Lines       37462    37463       +1     
==========================================
+ Hits        25744    25801      +57     
+ Misses       8435     8374      -61     
- Partials     3283     3288       +5
Impacted Files Coverage Δ
go/storage/mkvs/db/badger/badger.go 70.30% <100.00%> (+0.06%) ⬆️
go/oasis-node/cmd/common/metrics/disk.go 65.51% <0.00%> (-20.69%) ⬇️
go/oasis-node/cmd/common/metrics/resource.go 84.00% <0.00%> (-8.00%) ⬇️
go/storage/mkvs/syncer/proof.go 83.92% <0.00%> (-3.58%) ⬇️
go/consensus/tendermint/apps/staking/state/gas.go 81.03% <0.00%> (-3.45%) ⬇️
go/worker/storage/service_external.go 51.61% <0.00%> (-2.16%) ⬇️
go/storage/mkvs/iterator.go 83.12% <0.00%> (-1.25%) ⬇️
go/consensus/tendermint/abci/mux.go 73.50% <0.00%> (-1.16%) ⬇️
go/worker/compute/txnscheduler/committee/node.go 63.36% <0.00%> (-0.91%) ⬇️
go/runtime/client/client.go 72.99% <0.00%> (-0.73%) ⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 936912f...8c7734f. Read the comment docs.

@ptrus ptrus force-pushed the ptrus/feature/bump-badger branch 2 times, most recently from c678eb7 to fb13f7e Compare July 30, 2020 12:20
kostko
kostko approved these changes Aug 2, 2020
View reviewed changes
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 72h long-term tests shows no issues.

@kostko
Copy link
Member

kostko commented Aug 2, 2020 •
edited
Loading

Just noticed that Badger master has some additional fixes in comparison to rc3. Will run some tests using that (commit 3e067a54e5964bf7640c7d74fa92cd015f2a3fc8) as well.

@ptrus ptrus force-pushed the ptrus/feature/bump-badger branch from fb13f7e to 8c7734f Compare August 3, 2020 06:58
@ptrus
Copy link
Member Author

ptrus commented Aug 3, 2020

Ok, bumped badger in this PR to use that commit as well.

@ptrus ptrus marked this pull request as ready for review August 3, 2020 06:58
kostko
kostko approved these changes Aug 3, 2020
View reviewed changes
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long-term test on the new version is still running, but no issues so far. I'll keep it running but I think we can merge this.

@ptrus ptrus merged commit 4b70d95 into master Aug 3, 2020
@ptrus ptrus deleted the ptrus/feature/bump-badger branch August 3, 2020 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostko kostko kostko approved these changes

@peterjgilbert peterjgilbert Awaiting requested review from peterjgilbert peterjgilbert is a code owner

@pro-wh pro-wh Awaiting requested review from pro-wh pro-wh is a code owner

@Yawning Yawning Awaiting requested review from Yawning

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ptrus @kostko